Remove sidecars of annotated namespaces when annotation is deleted #1209
Conversation
…eleted Signed-off-by: Ruben Vargas <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1209 +/- ##
=======================================
Coverage 87.32% 87.32%
=======================================
Files 89 89
Lines 4891 4891
=======================================
Hits 4271 4271
Misses 457 457
Partials 163 163 Continue to review full report at Codecov.
|
jpkrohling
changed the title
| // If deployment don't have the annotation and has an agent, this may be injected by the namespace | ||
| // we need to clean it. | ||
| agent, _ := inject.HasJaegerAgent(dep) | ||
| _, depAnnotation := dep.Annotations[inject.Annotation] |
There was a problem hiding this comment.
s/depAnnotation/ok -- you are not getting the annotation, you are getting a boolean on whether it was found.
| agent, _ := inject.HasJaegerAgent(dep) | ||
| _, depAnnotation := dep.Annotations[inject.Annotation] | ||
| if agent && !depAnnotation{ | ||
| jaegerInstance, label := dep.Labels[inject.Label] |
| "jaeger": jaegerInstance, | ||
| }).Info("Removing Jaeger Agent sidecar") | ||
| inject.CleanSidecar(jaegerInstance, dep) | ||
| if err := r.client.Update(ctx, dep); err != nil { |
There was a problem hiding this comment.
Use Patch instead of Update, to reduce the risk of failure. Look at this one for reference: open-telemetry/opentelemetry-operator#54
Signed-off-by: Ruben Vargas <[email protected]>
| log.WithFields(log.Fields{ | ||
| "deployment": dep.Name, | ||
| "namespace": dep.Namespace, | ||
| "jaeger": jaegerInstance, | ||
| }).Info("Removing Jaeger Agent sidecar") | ||
| patch := client.MergeFrom(dep.DeepCopy()) |
There was a problem hiding this comment.
If dep is a pointer, you might not need to do a DeepCopy, you can just dereference it.
|
Yes I did manual testing, with the steps detailed in the issue on how to reproduce. |
…aegertracing#1209) Signed-off-by: Ruben Vargas <[email protected]>
Fixes #906
Signed-off-by: Ruben Vargas [email protected]